fix(audioreactive): skip i2s_set_clk() for PDM mode on IDF 5.x - #5807

No actionable comments were generated in the recent review. 🎉

Walkthrough

PDM channel selection now uses side-specific ALL_RIGHT or ALL_LEFT formats. PDM initialization skips i2s_set_clk(), while non-PDM initialization retains clock setup and failure handling.

Changes

PDM I2S configuration

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to da6a6

The change is limited to PDM microphone clock and channel-format handling; no actionable merge-blocking risk remains beyond normal checks and review.

Possibly related PRs

• wled/WLED#5357: Modifies overlapping I2S/PDM channel configuration in audio_source.h.
• wled/WLED#5764: Modifies overlapping PDM channel configuration and clock initialization in audio_source.h.

Suggested labels: bug, usermod

Suggested reviewers: softhack007

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

_{Comment @coderabbitai help to get the list of available commands.}

@coderabbitai is this PR compatible with upcoming changes in #5764 and #5773 ?

#!/bin/bash
set -euo pipefail

repo="wled/WLED"

for pr in 5807 5764 5773; do
  echo "===== PR #${pr}: metadata ====="
  gh pr view "$pr" --repo "$repo" --json number,title,state,isDraft,baseRefName,headRefName,mergeable,mergeStateStatus,url,body \
    --jq '{number,title,state,isDraft,baseRefName,headRefName,mergeable,mergeStateStatus,url,body}'
  echo "===== PR #${pr}: changed files ====="
  gh pr view "$pr" --repo "$repo" --json files \
    --jq '.files[] | {path, additions, deletions}'
  echo "===== PR #${pr}: diff ====="
  gh pr diff "$pr" --repo "$repo" --patch
done

echo "===== Local source outline ====="
ast-grep outline usermods/audioreactive/audio_source.h --items all --view expanded

echo "===== Relevant local I2S configuration ====="
rg -n -C 8 'I2S_PDM_MIC_CHANNEL|i2s_set_clk\s*\(|I2S_MODE_PDM|I2S_CHANNEL_FMT_(ONLY|ALL)_(LEFT|RIGHT)' \
  usermods/audioreactive/audio_source.h

@softhack007 No. PR #5807 is not directly merge-compatible with #5764.

• Both PRs modify usermods/audioreactive/audio_source.h.
• PR #5764 replaces the ESP-IDF 5.x legacy I2S driver with i2s_std.h and i2s_pdm.h.
• Therefore, #5807’s i2s_set_clk() guard does not apply after #5764. The new I2S API does not use i2s_set_clk().
• The PDM channel-format change also needs a separate review or port. #5764 uses i2s_pdm_rx_config_t, not I2S_CHANNEL_FMT_ALL_LEFT or I2S_CHANNEL_FMT_ALL_RIGHT.

PR #5807 remains relevant for the legacy IDF 4.x path in #5764. It must be manually rebased and resolved after #5764. The PDM microphone must then be tested on the new IDF 5.x driver path.

PR #5773 does not overlap with #5807. It adds the ADC manager and does not change the I2S or PDM code. It is compatible with #5807 in isolation.

_{You are interacting with an AI system.}

@coderabbitai your .coderabbit.yaml explicitly asks to check for deleted comments in AI generated source code. Why did you miss the two deleted comments in this PR?

#!/bin/bash
set -euo pipefail

echo '===== Configuration files ====='
fd -HI '^\.coderabbit\.(ya?ml)$|^AGENTS\.md$' . -d 3 -x sh -c 'echo "--- $1"; cat -n "$1"' sh {}

echo '===== PR `#5807` deleted lines and context ====='
git diff --find-renames --unified=12 origin/"$(git remote show origin | sed -n '/HEAD branch/s/.*: //p')"...HEAD -- usermods/audioreactive/audio_source.h \
  || true

echo '===== Relevant markers and comments in PR file ====='
rg -n -C 5 'AI:|I2S_PDM_MIC_CHANNEL|i2s_set_clk|//|/\*|\*/' usermods/audioreactive/audio_source.h

@softhack007 You are correct. I missed a required review check.

.coderabbit.yaml Line 58 applies extra checks when source appears AI-generated. Line 65 requires a finding for deleted comments that are unrelated to the PR description.

PR #5807 deletes these two comments from the i2s_set_clk() error path:

• // set bit clocks. Also takes care of MCLK routing if needed.
• // uninstall already-installed driver

I focused on the functional PDM change and did not perform the required removed-line review as a separate step. That omission caused the missed finding. The deleted comments should have been reviewed and either retained, moved to the new guarded path, or justified in the PR description.

🐇‍⬛

Learnt from: CR
Repo: wled/WLED PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-07-26T17:47:14.996Z
Learning: Applies to wled00/**/*.{cpp,h} : AI-generated C++ source blocks must be marked with `// AI: below section was generated by an AI` and `// AI: end`.

_{You are interacting with an AI system.}

Just some comments before I push fixup commit :

IDF 4.x compatibility:
Safe on all 4.x. i2s_driver_install() configures PDM clocks internally on every version -- the extra i2s_set_clk() is always redundant for PDM. On 4.4 it can actually crash if called with rate=0 (ESP-IDF #8850). Tested on IDF 5.x with SPM1423.

#5764 relationship:
This fixes the current legacy path. #5764 keeps that path for IDF 4.x, so the guard stays relevant. If the legacy path goes away entirely, the guard becomes dead code. PDM is broken on IDF 5.x today and #5764 is a bigger piece of work

@aenertia so you are saying PDM is broken in 16.0 and this fix needs a backport?

This fixes the current legacy path. #5764 keeps that path for IDF 4.x, so the guard stays relevant.

what do you mean by this then?